Skip to content

fixed issue in request method in case if this.restClientConfig is not provided in config - #237

Merged
AmsterGet merged 1 commit into
reportportal:developfrom
AlexGalichenko:fix-custom-agent
Oct 29, 2025
Merged

fixed issue in request method in case if this.restClientConfig is not provided in config#237
AmsterGet merged 1 commit into
reportportal:developfrom
AlexGalichenko:fix-custom-agent

Conversation

@AlexGalichenko

@AlexGalichenko AlexGalichenko commented Oct 29, 2025 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

fixed the issue in the request method in case if this.restClientConfig is not provided in the config

image

Summary by CodeRabbit

  • Bug Fixes
    • Fixed a potential runtime error in REST client configuration handling that could occur when no custom agent settings are provided.

@coderabbitai

coderabbitai Bot commented Oct 29, 2025 •
edited
Loading

Copy link
Copy Markdown

Walkthrough

The change adds a guard check to restClientConfig in lib/rest.js to prevent runtime errors when evaluating property existence. The fix ensures hasCustomAgents is false when no config is provided, altering proxy agent selection logic to use getProxyAgentForUrl in those cases.

Changes

Cohort / File(s) Summary
Guard clause fix for restClientConfig
lib/rest.js		 Added null/falsy check before accessing restClientConfig properties to prevent runtime errors when checking for custom agent keys. Changes proxy agent selection: uses getProxyAgentForUrl when no config exists, otherwise uses empty object.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • The fix is localized to a single guard check, but verifying the behavioral change to proxy agent selection warrants attention
  • Confirm that using getProxyAgentForUrl when restClientConfig is absent aligns with intended proxy handling
  • Test coverage for the undefined/falsy restClientConfig scenario should be verified

Poem

🐰 A guard clause guards the way,
Where configs once would surely fray,
No more shall undefined things fall,
The proxy agents heed the call! 🛡️
Safety wrapped in logical care.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "fixed issue in request method in case if this.restClientConfig is not provided in config" is directly related to the main change in the changeset. According to the summary, the fix guards against a runtime error by checking that restClientConfig exists before accessing its properties in the request() method. The title accurately captures this core fix and is specific enough that a teammate scanning the history would understand that this PR addresses a safeguard when restClientConfig is missing or undefined. While the phrasing has minor grammatical awkwardness, the meaning is clear and the title meaningfully describes the primary change.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b86e7f9 and f660936.

📒 Files selected for processing (1)
  • lib/rest.js (1 hunks)
🔇 Additional comments (1)
lib/rest.js (1)

70-71: Fix correctly prevents runtime error and handles undefined config gracefully.

The guard on lines 70-71 correctly prevents a TypeError when restClientConfig is undefined. The getProxyAgentForUrl function in lib/proxyHelper.js:196 has a default parameter (restClientConfig = {}), so when undefined is passed, it safely defaults to an empty object. This means the fix is complete and production-ready.

The optional chaining refactor suggested in the previous comment is not necessary—the current approach is clear and works correctly.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@AmsterGet

AmsterGet commented Oct 29, 2025

Copy link
Copy Markdown
Member

Ah, thank you @AlexGalichenko !

@AmsterGet
AmsterGet merged commit 2c6b732 into reportportal:develop Oct 29, 2025
7 checks passed
AmsterGet added a commit that referenced this pull request Oct 29, 2025
@AmsterGet @AlexGalichenko
Release 5.5.1 (#238)
03b7ef8 
* 5.5.0 -> 5.5.1-SNAPSHOT

* Merge pull request #237 from AlexGalichenko/fix-custom-agent

fixed issue in request method in case if this.restClientConfig is not provided in config

* Update changelog

---------

Co-authored-by: reportportal.io <reportportal.io>
Co-authored-by: Alexander Galichenko <alexandr.galichenko@gmail.com>
@coderabbitai coderabbitai Bot mentioned this pull request Nov 13, 2025
Merged
@coderabbitai coderabbitai Bot mentioned this pull request Dec 12, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AmsterGet AmsterGet AmsterGet approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@AlexGalichenko @AmsterGet